Repository navigation
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueNo actionable comments were generated in the recent review. ℹ️ Recent review info
Walkthrough
Merge Risk:
|
|
I investigated the two current red jobs; neither failure is caused by this PR's PostgreSQL tooling change:
I reproduced the second failure directly against the currently published image and reopened pulp-oci-images #115 until the stable tag is rebuilt with the already released packaging fix. No code change to this PR is indicated by either failure. Once the full workflow finishes and the stable Pulp tag is repaired, I will trigger a fresh matrix run. The PR's own lint and the live backup/restore validation remain green. |
|
The remaining red matrix is now covered by the shared follow-ups rather than this PostgreSQL-container change: #910 handles recovered Dynflow workers, #893 covers the CentOS Stream 9 fapolicyd cache, #863 fixes the FIPS/backup privilege-escalation timeout, and pulp-oci-images#115 tracks the broken 3.19 Pulp image used by the upgrade job. This PR’s lint, Packit, external-database, CentOS Stream 10 security, capsule and targeted live backup/restore validation are green. |
cc487d6 to
047e8ed
Compare
|
Note that #605 talks about an example, but that we should consider how to address the use of PostgresSQL tools inside the container as a whole. This only touches a subset of that and does it in a very brute force method and does not consider why we currently use the For me, this is an example of an issue that needs design discussion, not just trying to throw an agentic solution at it. |
|
I focused too narrowly on dump/restore. The design needs to cover database/user/extension management and readiness checks as well, and retain the community.postgresql modules rather than replace their behavior with repeated CLI commands. I would start by agreeing on a shared container execution mechanism for those modules and the few CLI-only operations, including internal/external databases, credentials and air-gapped image selection. This PR is not that complete solution yet, so I have left the implementation unchanged rather than treating another CI rerun as an answer to the design concern. |
047e8ed to
4eb6b8b
Compare
|
Fixed the backup test setup in 4eb6b8b. The IOP-enabled run had 466 passing tests but 14 backup setup errors because Also replaced the removed CI follow-up (28 October): the completed run had two failing jobs:
The other 22 jobs passed. I refreshed the unchanged code as |
Run readiness checks, dumps and restores in the configured PostgreSQL image instead of requiring host client packages. Keep backup tests usable when an existing installation already has those packages installed. Use login_db for postgresql_ext, supported since collection 4.0 and required after its old db alias was removed. Fixes theforeman#605 AI assistance: Codex. Project signature: GPT_5.6 Sol.
4eb6b8b to
7018c98
Compare
Why are you introducing these changes? (Problem description, related links)
Backup and restore should use PostgreSQL client tools that match the configured server image, without requiring PostgreSQL packages on the target host. This also works when host clients are already present, for example because another component installed them.
Fixes #605.
What are the changes introduced in this pull request?
pg_isready,pg_dumpandpg_restorein the configured PostgreSQL image with host networking. Mount the backup directory only for dump/restore operations and pass the password through the container environment.login_dbparameter forpostgresql_ext. Collection 4.0 supports it; collection 5 removed the olddbalias. This clears the restore syntax warning without raising the minimum supported collection version.How to test this pull request
Run the normal backup integration suite against internal and external databases, including IOP-enabled deployments. Verify the custom-format dumps and restore them in a disposable deployment. Test hosts with and without native PostgreSQL clients; both are supported.
Verification after rebasing on current
master:tests/,src/,development/scripts/andinventories/: passed.git diff --check: passed.Earlier live qualification exercised backup and restore without host clients: deployment and post-restore redeployment completed successfully; 76 targeted application/health tests passed with one skip. That earlier result is not a deployment test of the current revision. The current complete deployment/backup matrix runs in CI; no new local VM or image was installed for this update.
Checklist
AI assistance: Codex. Project signature: GPT_5.6 Sol.